Conversation
✅ Deploy Preview for astro-docs-2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
ArmandPhilippot
left a comment
There was a problem hiding this comment.
Thanks Ema! I left a few suggestions mostly around:
- docs style, consistency with other experimental pages
- and some reorganization to make it easier to scan. For example I find the API block placement a bit annoying currently:
Also, I noticed you mentioned the AstroLoggerDestination but I don't think we document its exact shape at the moment. It would be helpful to have a "Types reference" section with the public types/types required to build a custom logger.
Then, when the feature becomes stable we only need to reorganize the contents, everything would already be here.
For example, the experimental Route caching has an API reference but we could also use the same format used in the other references (e.g. Renderer types).
I was hoping to get some suggestions on where to have it, and if it needs an header |
|
If there is only one useful type, a reference section might not be the best approach and we could reshape
(we don't necessarily need to show Otherwise, a new section at the end might be easier to organize the content. Each section only describe what is available and we add links to the relevant types for advanced users. This remains easy to scan without having to scroll too much for those not interested in building their own logger. Something like:
And we have precedents for both approaches, so the structure remains familiar to users. |
|
Hey @ArmandPhilippot , I added more information to the page. Let me know what you think |
ArmandPhilippot
left a comment
There was a problem hiding this comment.
Thanks for the updates, @ematipico !
I left a few suggestions, most of them try to shorten/simplify the wording or add links when we can. And one of them is pretty big: if we replace your list with sections, we can link to them directly when needed and this might be easier to scan.
Feel free to disagree. 😄 After that, it looks ready to me!
|
|
||
| ### `logHandlers.compose` | ||
|
|
||
| A particular function that allows using multiple loggers. The same message is broadcasted to all loggers. The function accepts an arbitrary number of logger configurations.logHandlers |
There was a problem hiding this comment.
I think your autocomplete function is playing tricks on us again (.logHandlers) 😄 But I think we can combine the first and last sentence:
| A particular function that allows using multiple loggers. The same message is broadcasted to all loggers. The function accepts an arbitrary number of logger configurations.logHandlers | |
| A particular function that allows configuring multiple loggers in an arbitrary order. The same message is broadcasted to all loggers. |
| }); | ||
| ``` | ||
|
|
||
| The following example implements a minimal logger returning an `AstroLoggerDestination` object with the required `write()` function: |
There was a problem hiding this comment.
Since we now have a link I think this could be helpful here:
| The following example implements a minimal logger returning an `AstroLoggerDestination` object with the required `write()` function: | |
| The following example implements a minimal logger returning an [`AstroLoggerDestination` object](#astrologgerdestination) with the required `write()` function: |
| }); | ||
| ``` | ||
|
|
||
| We will expose an utility form the `astro/logger` package to check the log level. |
There was a problem hiding this comment.
From the changeset, this is already available, and we avoid "we" 😄
| We will expose an utility form the `astro/logger` package to check the log level. | |
| The `astro/logger` package exposes a [`matchesLevel()`](#matcheslevel) helper to check the log level. This can be useful when [building a custom logger](#custom-loggers). |
| This is the interface that custom loggers must implement. The following can be implemented: | ||
| - `write(message: AstroLoggerMessage) => void`: a mandatory method that accepts a `AstroLoggerMessage`. It's called for every log. | ||
| - `flush() => Promise<void> | void`: an optional function that is called at end of each request. Useful for advanced loggers for flushing logger messages while keeping the connection to the destination alive. | ||
| - `close() => Promise<void> | void`: an optional function that is called before a server is shut down. This is a function usually called by adapters such as `@astrojs/node`. |
There was a problem hiding this comment.
Nothing really wrong but:
- with sections, this is easier to scan and this allows us to use links to these sections.
- I simplified a bit some sentences (e.g.
function that is called>function called) to make them look like definitions
| This is the interface that custom loggers must implement. The following can be implemented: | |
| - `write(message: AstroLoggerMessage) => void`: a mandatory method that accepts a `AstroLoggerMessage`. It's called for every log. | |
| - `flush() => Promise<void> | void`: an optional function that is called at end of each request. Useful for advanced loggers for flushing logger messages while keeping the connection to the destination alive. | |
| - `close() => Promise<void> | void`: an optional function that is called before a server is shut down. This is a function usually called by adapters such as `@astrojs/node`. | |
| This is the interface that custom loggers must implement. | |
| #### `AstroLoggerDestination.write()` | |
| <p> | |
| **Type:** `(message: AstroLoggerMessage) => void` | |
| </p> | |
| A mandatory method called for each log and accepting an [`AstroLoggerMessage`](#astrologgermessage). | |
| #### `AstroLoggerDestination.flush()` | |
| <p> | |
| **Type:** `() => Promise<void> | void` | |
| </p> | |
| An optional function called at the end of each request. This is useful for advanced loggers that need to flush log messages while keeping the connection to the destination alive. | |
| #### `AstroLoggerDestination.close()` | |
| <p> | |
| **Type:** `() => Promise<void> | void` | |
| </p> | |
| An optional function called before a server is shut down. This function is usually called by adapters such as `@astrojs/node`. |
For the last sentence I hesitated to simplify it:
-An optional function called before a server is shut down. This function is usually called by adapters such as `@astrojs/node`.
+An optional function called by adapters (e.g. `@astrojs/node`) before shutting down the server.But maybe "usually" was intentional so I haven't updated that.
|
|
||
| ### `AstroLoggerMessage` | ||
|
|
||
| The incoming object from the `AstroLoggerDestination.write` function: |
There was a problem hiding this comment.
With the previous suggestion, we can now add a link here:
| The incoming object from the `AstroLoggerDestination.write` function: | |
| The incoming object from the [`AstroLoggerDestination.write()`](#astrologgerdestinationwrite) function: |
| ### `AstroLoggerMessage` | ||
|
|
There was a problem hiding this comment.
Is inlining the type useful here? I mean I noticed you specified that message is a string below and I see that label can be null
| ### `AstroLoggerMessage` | |
| ### `AstroLoggerMessage` | |
| <p> | |
| **Type:** `{ label: string | null; level: AstroLoggerLevel; message: string; newLine: boolean; }` | |
| </p> |
| ### `AstroLoggerMessage` | ||
|
|
||
| The incoming object from the `AstroLoggerDestination.write` function: | ||
| - `message: string`: the message being logged. |
There was a problem hiding this comment.
If my previous suggestion makes sense we no longer need this and all items are consistent:
| - `message: string`: the message being logged. | |
| - `message`: the message being logged. |
|
|
||
| ### `matchesLevel` | ||
|
|
||
| `matchesLevel(messageLevel: AstroLoggerLevel, configuredLevel: AstroLoggerLevel): boolean` |
There was a problem hiding this comment.
nit, formatting:
| `matchesLevel(messageLevel: AstroLoggerLevel, configuredLevel: AstroLoggerLevel): boolean` | |
| <p> | |
| **Type:** `matchesLevel(messageLevel: AstroLoggerLevel, configuredLevel: AstroLoggerLevel) => boolean` | |
| </p> |
|
|
||
| `matchesLevel(messageLevel: AstroLoggerLevel, configuredLevel: AstroLoggerLevel): boolean` | ||
|
|
||
| Given two [logger level](#log-level), it returns whether the first level matches the second level. |
There was a problem hiding this comment.
| Given two [logger level](#log-level), it returns whether the first level matches the second level. | |
| Given two [log levels](#log-level), it returns whether the first level matches the second level. |


Description (required)
This PR adds documentation for the upcoming experimental logger. The RFC is here: withastro/roadmap#1339
Linear: AST-84
For Astro version:
6.2.0. See astro PR #16437.The attached PR is the one that mostly documents the new APIs